Skip to content

BE-319: Split backend integration tests into seeded and snapshot groups#9006

Merged
thehabbos007 merged 2 commits into
mainfrom
t/be-319-destructive-test-registration
Jul 14, 2026
Merged

BE-319: Split backend integration tests into seeded and snapshot groups#9006
thehabbos007 merged 2 commits into
mainfrom
t/be-319-destructive-test-registration

Conversation

@claude

@claude claude Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Requested by Tim Diekmann, Ahmad Sattar Atta · Slack thread

🌟 What is the purpose of this PR?

Follow-up to #9001, addressing review feedback: the boundary between tests that rely on the shared seeded graph and tests that wipe it was a path-prefix convention inside a single vitest run.

Before: one vitest run covered all test files, and a custom sequencer (DestructiveTestsLastSequencer) identified destructive files by the src/tests/subgraph/ path prefix and sorted them last. A destructive test added elsewhere would silently run mid-pack, wipe the graph seeded once per run by globalSetup, and corrupt the shared seed for every test file after it — the ordering was a convention, not a guarantee.

After: the suite is split into two explicit groups that run as separate, sequential vitest invocations:

  • the seeded group (vitest.config.ts, src/tests/graph/) keeps the globalSetup seed and structurally cannot wipe the graph — its invocation contains no destructive file and completes before the destructive run starts;
  • the snapshot group (vitest.snapshot.config.ts, src/tests/subgraph/) owns the wipe: it has no globalSetup, and its files reset the graph and restore standalone snapshots;
  • a runtime guard enforces the boundary: resetGraph/restoreSnapshot throw a clear error unless the snapshot group's config marker is present, so a destructive call from the seeded group fails immediately in the offending file instead of corrupting unrelated tests.

The sequencer and the central registry file are gone; the groups are defined by the two configs over the existing directory layout (no test files were moved).

🔗 Related links

🔍 What does this change?

All changes are in tests/hash-backend-integration:

  • vitest.config.ts (seeded group): include is now only src/tests/graph/**/*.test.ts (18 files); keeps globalSetup. The DestructiveTestsLastSequencer and the src/tests/destructive-test-files.ts registry are deleted. Options common to both groups are exported as sharedTestConfig.
  • vitest.snapshot.config.ts (new, snapshot group): includes only src/tests/subgraph/**/*.test.ts (3 files: circular, friendship, simple); no globalSetup; sets the HASH_TEST_GROUP=snapshot marker via vitest's test.env; writes coverage to ./coverage-snapshot so it doesn't clean the seeded group's ./coverage report (codecov's uploader discovers both).
  • package.json: test:integration is now vitest --run && vitest --run --config vitest.snapshot.config.ts — seeded group first, snapshot group second. CI is unchanged: .github/workflows/test.yml invokes this via turbo run test:integration.
  • src/tests/admin-server.ts: resetGraph() and restoreSnapshot() call assertRunningInSnapshotGroup, which throws unless process.env.HASH_TEST_GROUP === "snapshot". The error explains that destructive graph operations only run in the snapshot group and that a test file becomes destructive by placing it under src/tests/subgraph/. Other admin helpers (e.g. deleteUser, used by the seeded user.test.ts) are unguarded, as before. The module no longer imports from vitest, so it is usable outside a vitest worker again (no other consumers exist today — verified by grep).
  • tsconfig.json: adds the new config file to include so tsc/typed eslint cover it.

Guard mechanism: in vitest 4.1.8, test.env from the config verifiably reaches process.env in test workers before any test file runs: the pool serializes env: { ...viteConfig.env, ...config.env } into the worker context, and the worker's setupCommonEnv writes each key through the metaEnv proxy, whose set trap assigns process.env[key] (verified in the installed vitest/dist/chunks/cli-api.*.js, setup-common.*.js, and init.*.js, and confirmed empirically with a scratch vitest run against the repo's installed vitest). In the seeded group's invocation the marker is simply never set, so the guard throws there.

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

⚠️ Known issues

  • With coverage enabled, reports now land in two directories (coverage/ from the seeded run, coverage-snapshot/ from the snapshot run); the codecov action auto-discovers lcov files, so both are uploaded.
  • src/tests/email.test.ts is matched by neither config — unchanged from main, whose include patterns (graph/** + subgraph/**) also excluded it.

🛡 What tests cover this?

The backend integration suite itself (yarn test:integration in tests/hash-backend-integration): the seeded group exercises the shared seed on every run, and the three snapshot-group files exercise the guard's pass path on every run.

Verification performed:

  • tsc --noEmit and eslint for @tests/hash-backend-integration: the full dependency graph (wasm build, graph client codegen) cannot be built in my sandbox, so both were run on this branch and on main in the same unbuilt-deps state and the outputs diffed — byte-identical (pre-existing errors from unbuilt workspace deps on both; zero findings introduced by this change). oxfmt --check passes on all five changed files.
  • vitest list --filesOnly run with both configs proves the partition: the seeded config lists exactly the 18 src/tests/graph/** files, the snapshot config lists exactly the 3 src/tests/subgraph/** files — no overlap, and together they cover every file the single config on main included.
  • The test.envprocess.env mechanism was verified in the installed vitest 4.1.8 dist and confirmed empirically (a scratch test asserting process.env.HASH_TEST_GROUP === "snapshot" passes under a config setting it via test.env).
  • Not verified locally: an actual run of the suite — starting the graph stack is blocked in my sandbox (same limitation as BE-319: Seed the backend integration test graph once per run #9001). CI on this PR is the real verification.

❓ How to test this?

  1. Checkout the branch
  2. Start external services and the graph as usual, then yarn workspace @tests/hash-backend-integration test:integration — the seeded group runs first against the globalSetup seed, then the snapshot group runs the three subgraph/ files in a second vitest invocation.
  3. To see the guard: add an await resetGraph() to any seeded-group test file — it fails immediately with an error explaining that destructive graph operations only run in the snapshot group (vitest.snapshot.config.ts) and how to place a test there.

Generated by Claude Code

@vercel

vercel Bot commented Jul 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Actions Updated (UTC)
hash Ignored Ignored Preview Jul 14, 2026 7:47am
hashdotdesign-tokens Ignored Ignored Preview Jul 14, 2026 7:47am
petrinaut Skipped Skipped Jul 14, 2026 7:47am

@github-actions github-actions Bot added area/tests New or updated tests area/tests > integration New or updated integration tests labels Jul 10, 2026
@claude claude Bot marked this pull request as ready for review July 10, 2026 12:29
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Changes are confined to integration test harness and guards; production graph/API behavior is unchanged, with clearer isolation between seeded and destructive test runs.

Overview
Replaces a single Vitest run plus DestructiveTestsLastSequencer (ordering src/tests/subgraph/ last) with two explicit, sequential runs: seeded tests (vitest.config.ts, src/tests/graph/**, keeps globalSetup) then snapshot tests (vitest.snapshot.config.ts, src/tests/subgraph/**, no globalSetup). Shared options live in sharedTestConfig; test:integration chains both configs.

resetGraph and restoreSnapshot in admin-server.ts now call assertRunningInSnapshotGroup, which requires HASH_TEST_GROUP=snapshot (set only in the snapshot config). Misuse from the seeded group fails fast with guidance to put destructive tests under src/tests/subgraph/. Snapshot coverage writes to ./coverage-snapshot so the seeded run’s report is not wiped.

Reviewed by Cursor Bugbot for commit 09ffde8. Bugbot is set up for automated code reviews on this repo. Configure here.

@claude claude Bot requested review from TimDiekmann and thehabbos007 July 10, 2026 12:29
The seeded group (vitest.config.ts, src/tests/graph/) runs against the
system graph seeded once per run by globalSetup and must never wipe it.
The snapshot group (vitest.snapshot.config.ts, src/tests/subgraph/)
wipes the graph and restores standalone snapshots, so test:integration
runs it as a separate vitest invocation after the seeded group. A
runtime guard in admin-server.ts makes resetGraph/restoreSnapshot
refuse to run outside the snapshot group, keyed off the HASH_TEST_GROUP
marker set by the snapshot config.
@claude claude Bot force-pushed the t/be-319-destructive-test-registration branch from 165923d to 05656ce Compare July 10, 2026 13:12
@vercel vercel Bot temporarily deployed to Preview – petrinaut July 10, 2026 13:12 Inactive
@claude claude Bot changed the title BE-319: Register destructive integration tests explicitly and guard graph resets BE-319: Split backend integration tests into seeded and snapshot groups Jul 10, 2026

@thehabbos007 thehabbos007 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but i feel like the comments are overly verbose, when what they describe boils down to some very simple primitives and values. Be more succinct, otherwise this looks fine to me.

@vercel vercel Bot temporarily deployed to Preview – petrinaut July 14, 2026 07:46 Inactive
@thehabbos007 thehabbos007 added this pull request to the merge queue Jul 14, 2026
Merged via the queue into main with commit e3768e4 Jul 14, 2026
41 checks passed
@thehabbos007 thehabbos007 deleted the t/be-319-destructive-test-registration branch July 14, 2026 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/tests > integration New or updated integration tests area/tests New or updated tests

Development

Successfully merging this pull request may close these issues.

2 participants